Skip to content

Add conditional permission support - #886

Draft
Malika7188 wants to merge 7 commits into
tchx84:masterfrom
Malika7188:feature/conditional-permissions-marker
Draft

Malika7188 wants to merge 7 commits into
tchx84:masterfrom
Malika7188:feature/conditional-permissions-marker

Conversation

@Malika7188

@Malika7188 Malika7188 commented Aug 15, 2026 •

Copy link
Copy Markdown
Contributor

Previously, conditional permission entries (if:...) were discarded
entirely while parsing, to avoid treating them as unsupported and
corrupting the overrides file.

This now loads the underlying permission through the normal path,
the same as any other option, and separately records the raw
condition. A new status icon marks a permission row as conditional,
with a tooltip stating the condition it depends on.

Limitations
Flatpak drops a conditional permission if a bare grant for the same
permission is applied afterward, so the switch showing "on" doesn’t
always mean the permission is actually granted at runtime.
This is Flatpak’s behaviour and not something Flatseal controls.

saveToKeyFile right now only writes from the overrides Set (the on/off state)
and doesn’t look at the conditionals map. So saving can drop an existing
conditional entry from the override file. Write-back is not implemented yet

@Malika7188
Malika7188 force-pushed the feature/conditional-permissions-marker branch from 8369490 to d71264f Compare August 15, 2026 01:03
@Malika7188
Malika7188 marked this pull request as draft August 15, 2026 01:04
@Malika7188
Malika7188 force-pushed the feature/conditional-permissions-marker branch from d71264f to a79d0a4 Compare August 15, 2026 01:16
  Previously, conditional permission entries (if:...) were discarded
  entirely while parsing, to avoid treating them as unsupported and
  corrupting the overrides file.

  This now loads the underlying permission through the normal path,
  the same as any other option, and separately records the raw
  condition. A new status icon marks a permission row as conditional,
  with a tooltip stating the condition it depends on
@Malika7188
Malika7188 force-pushed the feature/conditional-permissions-marker branch from a79d0a4 to 84181ad Compare August 17, 2026 14:57

@tchx84 tchx84 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides the other comments, in order to design this in a future-proof / defensive way, imagine if tomorrow some other permission adds support for conditional but Flatseal doesn't support it, what Flatseal's behavior should be?

Comment thread src/models/permissions.js Outdated
Comment thread src/models/permissions.js Outdated
@Malika7188
Malika7188 force-pushed the feature/conditional-permissions-marker branch from c69d942 to 6abc833 Compare August 22, 2026 23:09
@Malika7188

Copy link
Copy Markdown
Contributor Author

Besides the other comments, in order to design this in a future-proof / defensive way, imagine if tomorrow some other permission adds support for conditional but Flatseal doesn't support it, what Flatseal's behavior should be?

I’ve fixed this. If a future permission adds conditional support that Flatseal doesn’t recognize yet, it now falls into the same unsupported bucket as a normal unrecognized permission and is preserved as it is, including the if: prefix. This way, it can be written back unchanged when saving instead of losing the condition.

Comment thread src/models/permissions.js
Comment thread src/models/permissions.js Outdated
Comment thread src/models/permissions.js Outdated
Comment thread tests/src/testModels.js Outdated
Per-app conditional permissions were silently dropped on unrelated
saves instead of being preserved.

Preserve a per-app conditional exactly as it was loaded unless the
option's current value no longer matches what the conditional's own
bare entry implies. In that case, the user's explicit toggle wins,
matching how a fresh bare grant clears a conditional in Flatpak.

Bump ecmaVersion to 2020 for optional chaining.
Comment thread src/models/permissions.js Outdated
Comment thread src/models/permissions.js Outdated
Comment thread src/models/shared.js Outdated
Comment thread tests/content/user/flatpak/overrides/com.test.Unsupported Outdated
Comment thread .eslintrc.yml Outdated
Comment thread tests/src/testModels.js
Comment thread src/models/permissions.js
Comment thread src/models/permissions.js Outdated
@Malika7188
Malika7188 force-pushed the feature/conditional-permissions-marker branch from 96d25cc to cec8a99 Compare September 8, 2026 22:06
Comment thread .eslintrc.yml Outdated
Recognized conditionals (on shared, sockets, devices, or features
permissions) are loaded and shown in the UI, each marked with the
condition it depends on. But saveToKeyFile only writes plain on/off
overrides, so the condition text is ignored when the override file
is saved.

Conditional permissions are safely ignored rather than being
preserved or modified.

Bumped ecmaVersion to 2020 for optional chaining.
@Malika7188
Malika7188 force-pushed the feature/conditional-permissions-marker branch from cec8a99 to 5c5b9a7 Compare September 21, 2026 08:45
@Malika7188

Copy link
Copy Markdown
Contributor Author

Hi @tchx84 , I've made the changes and removed the optional chaining. The ignore unsupported conditionals case is now its own check at the top of the block. I also reverted ecmaVersion back to 2018 in .eslintrc.yml. Ran the checks and everything still passes.

Also, I misunderstood your earlier comment about "marking the permission as optional." I thought you meant using optional chaining (?.), that's why I used it. Sorry about that confusion.

@tchx84 tchx84 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can still simplify this while making its scope more clear.

Comment thread tests/content/user/flatpak/overrides/com.test.Unsupported
Comment thread src/models/permissions.js
Comment thread src/models/permissions.js
Comment thread tests/src/testModels.js
Comment thread src/models/permissions.js
Comment thread src/models/permissions.js Outdated
Comment thread src/models/shared.js
* guarantee the permission is actually granted at runtime. */
markConditional(option, rawValue) {
this._conditionals.set(option, rawValue);
}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested the following scenario:

  1. Running latest version of this PR.
  2. Having installed your HelloConditional demo app.
  3. I launch Flatseal and select HelloConditional.
  4. I scroll down to the Socket section and negate PulseAudio.

What I see after that is:

Image

I am wondering about the semantics here; it definitely does make sense to display the "conditional" icon when the original permission is still valid but, after it's negated, does it still make sense to display it? Is it adding useful information ?

My first reaction is, probably not. Once we override, the "active" version of that permission is no longer the conditional but an explicit negation (not a conditional negation).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I've changed it so the conditional icon is hidden as soon as the permission is overridden, both by the user and globally. So in your PulseAudio example, only the override icon shows now. If the override is removed, the conditional icon comes back.

Comment thread src/style.css
row .conditional {
padding: 8px;
background-color: transparent;
background-image: -gtk-icontheme("dialog-question-symbolic");

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explore other icons and colors options for this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are some of the icon and color options I explored. Let me know what you think about them.

Option 1: The branch-arrow-symbolic choice is because it reads as “this depends on something,” which matches how a conditional permission works.

image

Option 2: The information icon (dialog-information-symbolic) in the default text color, instead of the current question-mark icon (which Flatseal also uses for help).
image


Option 3: The information icon (dialog-information-symbolic) in green
image

Comment thread tests/src/testModels.js
'if:unsupported-permission:!has-unsupported-permission')).toBe(false);
expect(has(
_unsupportedOverride, 'Context', 'unsupported',
'unsupported-permission')).toBe(false);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a check for the supported "always" permission and double check with a "hasOnly" 1, to explicitly describe the expected outcome and to double check that the unsupported conditional wasn't written back in a different way.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried hasOnly, but it fails here because the saved file also has shared and filesystems lines. Can I use has for always and hasInTotal to make sure nothing else was written, or do I give this test its own fixture so hasOnly works?

Comment thread src/models/permissions.js
GObject.signal_handler_block(this, this._notifyHandlerId);

Object.values(MODELS).forEach(model => model.updateStatusProperty(this));
CONDITIONAL_MODELS.forEach(model => model.updateConditionalProperty(this));

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another data point regarding conditionals models list; we add a lot of properties that we don't ever use.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated the implementation so that conditional properties are only created for the four models in CONDITIONAL_MODELS, so we no longer add properties that aren't used.

Comment thread src/models/permissions.js
Comment thread tests/src/testModels.js
permissionsDefault.appId = _conditionalAppId;

expect(permissionsDefault.features_devel).toBe(true);
expect(permissionsDefault.features_devel_conditional).toBe('');

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remind me what causes this devel conditional not getting marked? I don't see any global-related check in the parsing method.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added that to check that conditionals from global overrides are also dropped when they're detected.

Conditionals from an override (per-app or global) are never written
back, saveToKeyFile only writes the plain on/off state, so any
condition text from an override is silently dropped on save.

Displaying a marker for these was misleading, and the user would see a
conditional on load, only for it to disappear the next time anything
is saved. Only mark conditionals from the app's original metadata,
since those are not affected by saving overrides.

Added test coverage for both per-app and global override scenarios.
@Malika7188
Malika7188 force-pushed the feature/conditional-permissions-marker branch from 66779b5 to e996745 Compare October 2, 2026 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants